Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ui: remove experimental API in checkBrowser.js #776

Merged
merged 4 commits into from
Oct 22, 2020

Conversation

unbyte
Copy link
Contributor

@unbyte unbyte commented Oct 22, 2020

image


close #770

when testing the related issue on the low version of the Firefox, we found that alert from checkBrowser.js can't appear for the experimental API checkBrowser.js uses is not supported by these low version browsers.

@@ -40,7 +40,7 @@ function checkBrowser() {
d.getElementsByTagName('a')[0].onclick = function () {
d.getElementsByTagName('div')[0].style.top = '-60px'
}
document.body.prepend(d)
document.body.prepend ? document.body.prepend(d) : document.body.insertBefore(d, document.body.firstChild)
Copy link
Collaborator

@baurine baurine Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I prefer to directly use document.body.insertBefore(d, document.body.firstChild) because it can be used in all browsers.

We can use 1 PR to resolve the problems of #770 together (will discuss it later).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, however, the support for firstChild in chromes <=85 is unknown as caniuse shows

Copy link
Contributor

@kennytm kennytm Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how about childNodes[0].

but i highly doubt Chrome 85- not supporting firstChild which is in DOM 1 released back in 1998.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just curious about why support is unknown. maybe there is a special reason?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is fine, according to this doc: https://developer.mozilla.org/zh-CN/docs/Web/API/ParentNode/prepend , prepend polyfill is implemented by insertBefore and firstChild, even if document.body.firstChild is undefined, it works as expected, or we can consider using appendChild?

Copy link
Contributor Author

@unbyte unbyte Oct 22, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unknown comes from no real data yet. see mdn/browser-compat-data#6369
so yeah we can use insertBefore directly

@baurine
Copy link
Collaborator

baurine commented Oct 22, 2020

https://developer.mozilla.org/zh-CN/docs/Web/API/ParentNode/prepend

prepend can be used in chrome 54+, edge 17+, Firefox 49+

@unbyte
Copy link
Contributor Author

unbyte commented Oct 22, 2020

since Firefox 46 is not in our support list, dashboard still can't work in it. However, alert works well now.
image

Copy link
Collaborator

@baurine baurine left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@baurine baurine merged commit 9ecbccf into pingcap:master Oct 22, 2020
breezewish pushed a commit that referenced this pull request Nov 26, 2020
- remove experimental API in checkBrowser.js
- polyfill for Object.entries
breezewish added a commit that referenced this pull request Nov 26, 2020
* misc: Increase ulimit to 65535 for test env (#756)
* test: Fix frontend CI (#752)
* ui: fix dayjs i18n (#755)
* ui: handle error globally (#757)
* statement, slow_query: support all fields in list page (#749)
* ui: memorize expand/collapse full text in detail pages (#775)
* ui: break loop dependencies (#771)
* ui: fix browser compatibility check (#776)
* ui: Refine store location, add zoom and pan (#772)
* ui: show disk usage information for statement and slow query (#777)
* ui: use qps instead of ops (#786)
* statement: support export (#778)
*: Fix slow query and start_ts not working in some cases (#793)
* ui: fix errors doesn't display (#794)
* ui: fix the error message doesn't show correct (#799)
* slow_queries: support export (#792)
* ui: add MySqlFormatter to customize the sql formatter (#805)
*: fix query statement detail error cause by round (#806)
* ui: copy original content instead of formatted content for CopyLink (#802)
* add min height of topology canvas (#804)
* metrics: Support customize Prometheus address (#808)
* clusterinfo: Refine (#815)
* ui: Open statement and slow log in new tab (#816)
* ui: add more time field for slow query detail page (#810)
* slowlog: Improve descriptions (#817)
* build: add action to check release-version is changed for release branch
* Release v2020.11.26.1
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Check browerlist whether really works
5 participants